Add reserved-memory support #35460

carlocaione · 2021-05-19T14:30:43Z

This PR was inspired by #31077 (tagging @JordanYates)

What this RFC is about

This RFC is a potential answer to two different needs:

A driver needs to have exclusive access to a very specific memory region. For example frame-buffer area or some portion of memory at a very precise location that is shared among different SoCs on the same platform.
A driver needs to have access to a memory buffer mapped in memory with particular attributes (for example cacheability attributes) that can offer services to other drivers. For example some kind of driver managing a shared DMA memory pool region.

How this is solved on other OS
The Linux kernel is implementing the reserved-memory node for that, see the documentation

How this RFC is trying to solve this
The idea is having a light version of what Linux is doing. What I'm proposing here is formally introducing a reserved-memory node and a set of header files and macros doing the following:

For each reserved memory region in the DT a new memory region is created in the linker script using the DT_RESERVED_REGIONS() macro
For each reserved memory region in the DT a new section is created in the linker script using the DT_RESERVED_SECTIONS() macro
For each reserved memory region in the DT two new linker symbols are created marking the start and the end of the new section using the DT_RESERVED_SYMBOLS() macro

A couple of other DT_RESERVED_* macros are also introduced to retrieve at run-time information about start and size of the reserved-memory region.

Can you give me an example?
Sure. Let's say that we have this in the DT:

reserved-memory {
    compatible = "reserved-memory";
    #address-cells = <1>;
    #size-cells = <1>;
    ranges;

    res0: res0@42000000 {
        compatible = "dma-buffer-manager";
        reg = <0x42000000 0x1000>;
        label = "res0";
        non-cacheable;
        linker-section;
    };

    res1: res1@43000000 {
        reg = <0x43000000 0x1000>;
        label = "res1";
        no-mmu;
        linker-section;
    }
};

driver: driver {
    compatible = "driver";
    memory-region = <&res1>;
}

The dma-buffer-manager driver is managing the memory region 0x42000000-0x42001000 set with a non-cacheable attribute at boot time by MPU/MMU while the driver driver has access through the phandle to the memory region 0x43000000-0x43001000. These drivers can use the macros DT_RESERVED_GET_{SIZE,ADDR} and DT_RESERVED_GET{SIZE,ADDR}_BY_PHANDLE to access address and size of the appropriate memory region.

In the linker script two more regions are created res0 and res1 and two more sections .res0 and .res1. Also new linker script symbols are being exported reserved_res0_{start,end} and reserved_res1_{start,end}.

MEMORY
{
...
res0 (rw) : ORIGIN = 1107296256, LENGTH = 4096
res1 (rw) : ORIGIN = 1124073472, LENGTH = 4096
}

res0 : {
  reserved_res0_start = .;
  KEEP(*(.res0))
  KEEP(*(.res0.*))
  reserved_res0_end = reserved_res0_start + 4096;
} > res0

res1 : {
  reserved_res1_start = .;
  KEEP(*(.res1))
  KEEP(*(.res1.*))
  reserved_res1_end = reserved_res1_start + 4096;
} > res1

How this solves the original problem
Having a real mechanism to actually reserve pages is a complex matter that usually involves MMU / MPU. What we are doing here is creating new sections and allow driver to make use of these new sections at very precise location in memory. If the sections are overlapping with other sections at compile time we get an error.

Also (very important) having access to the linker symbols, easily allows architectural code to set properties on the memory regions. For example MMU and MPU code can set proper attributes to the memory regions.

For example we can now pass reserved_res0_start to a slab-allocator to be able to have easily slabs of non-cacheable memory.

mbolivar-nordic

No objections from me on the approach.

I know this is at RFC stage but I have random nitpicks and a couple of questions.

include/devicetree.h

scripts/dts/gen_defines.py

dts/bindings/reserved-memory/reserved-memory.yaml

mbolivar-nordic · 2021-05-19T15:41:35Z

include/devicetree/reserved.h

This will require an update to doc/reference/devicetree/api.rst as well.

include/devicetree/reserved.h

mbolivar-nordic · 2021-05-19T15:56:43Z

include/linker/devicetree_reserved.h

@tejlmand will need to review this to see how it's going to play with linker script generation, which is also a 2.7 feature.

@carlocaione my expectation is that you may need to lift this to cmake instead of .h at some point, based on what @tejlmand demonstrated at the TSC meeting today regarding toolchain abstractions and impact on linker scripts.

I'll let y'all sort out the details; I'm just calling attention to it.

I was not aware of @tejlmand work, thanks for pointing that out. Looking forward to @tejlmand comments then ;)

Indeed, macro generation of linker sections is going to clash directly with @tejlmand's work. But since this RFC is probably going to come in earlier than the linker script generation rework, we should make sure that @tejlmand takes this into account during his development.

I'm not so worried on this.
It would of require that i'm able to do similar, but as we anyway need to obtain device tree info for the linker script generation I don't have concerns in this regard.

galak · 2021-05-20T13:22:57Z

Why do we need zephyr,reserved?
Why do we need linker sections for this?
How does this play with the existing uses of reserved memory in devicetree already in tree today?

carlocaione · 2021-05-21T08:44:43Z

1. Why do we need `zephyr,reserved`?

To quickly access the node using DT_CHOSEN but I can probably remove that.

2. Why do we need linker sections for this?

Given that you could have this done using memory regions "far" from the used kernel space and accessing the memory using direct pointers in memory having sections is handy for several reasons:

If your reserved memory is overlapping with some other section you get an error at compile time
Having sections allows you to put variables in that section using __in_section()
Allows the code to create linker symbols and a set of helpers to easily manage the reserved region

3. How does this play with the existing uses of reserved memory in devicetree already in tree today?

The reserved-memory node today in Zephyr is not currently doing anything, it's just creating nodes to be used by some other code in an arbitrary way. It is also slightly misused because the sub-nodes are not defining reserved memory regions as but are mostly used to partition the space. We can make the approach in this PR co-exist with the current use by introducing a property in the nodes (for example something like create-section;) and create regions/sections/linker symbols only for the regions for which this property is defined, basically the equivalent of linker-region; property for #31077.

galak · 2021-05-21T10:34:14Z

1. Why do we need `zephyr,reserved`?
To quickly access the node using DT_CHOSEN but I can probably remove that.

Lets remove it.

mbolivar-nordic

Thanks for the updates!

include/devicetree.h

dts/bindings/reserved-memory/reserved-memory.yaml

carlocaione · 2021-05-25T07:12:42Z

@carlocaione -- it sounds like you're saying you prefer to keep the compatible?

Yes. For now I'd like to keep it.

JordanYates · 2021-05-25T07:33:02Z

If your reserved memory is overlapping with some other section you get an error at compile time

Just pointing out that this is not strictly true. It will only fail at link time if variables placed in two memory regions overlay one another. You can test this easily by just duplicating the FLASH memory region in linker.ld

carlocaione · 2021-05-25T07:36:57Z

Just pointing out that this is not strictly true. It will only fail at link time if variables placed in two memory regions overlay one another. You can test this easily by just duplicating the FLASH memory region in linker.ld

Thank you for the clarification. This is indeed what I see in my test where I usually create a buffer of the same size of the section to be used as memory pool. I can make that crated automatically, I'm open to any suggestion on how to improve this.

mbolivar-nordic

Thanks @carlocaione

carlocaione · 2021-07-01T08:39:05Z

@galak ping

carlocaione · 2021-07-06T08:09:58Z

Can we please merge this? @nashif or @galak?

@galak

dismissing @galak review because I addressed the comments long time ago already.

carlocaione · 2021-07-14T16:09:28Z

@carlescufi can we move this forward please? :)

samples/boards/qemu_cortex_a53/reserved_memory/dts/bindings/sample_driver.yaml

galak

So this looks good, my only concern is adding DT_RESERVED_MEM_REGIONS / DT_RESERVED_MEM_SECTIONS to linker.ld unconditionally.

As its reasonable for someone to have reserved memory nodes w/o labels and this would somewhat break things.

So do we add a Kconfig option to enable the support.

JordanYates · 2021-07-15T01:16:42Z

So this looks good, my only concern is adding DT_RESERVED_MEM_REGIONS / DT_RESERVED_MEM_SECTIONS to linker.ld unconditionally.
As its reasonable for someone to have reserved memory nodes w/o labels and this would somewhat break things.

One alternative is to generate the sections names from the node labels instead of the label property as in #36365.

To be able to get a tokenize DT string without the quotes. Deprecate also DT_ENUM_TOKEN and DT_ENUM_UPPER_TOKEN. Signed-off-by: Carlo Caione <[email protected]>

Introduce a set of header files to be able to define and declare sections and regions in the linker script. Introduce also DT helpers to retrieve data back. Signed-off-by: Carlo Caione <[email protected]>

carlocaione · 2021-07-15T07:32:41Z

So this looks good, my only concern is adding DT_RESERVED_MEM_REGIONS / DT_RESERVED_MEM_SECTIONS to linker.ld unconditionally.

This won't be a problem anymore (and it will be reworked) once the @tejlmand work on the linker script generation will be merged. It's really not useful trying to fix this sort of things now when the whole system will be reworked soon.

As its reasonable for someone to have reserved memory nodes w/o labels and this would somewhat break things.

As suggested by @JordanYates this can be fixed once his PR #36365 will be merged (and FWIW that approach looks sane to me).

So do we add a Kconfig option to enable the support.

Not clear what you are asking here, support for what?

carlescufi · 2021-07-15T11:48:22Z

@galak @JordanYates added this to dev-review with the hope that we can settle it today.

galak · 2021-07-15T12:30:22Z

So I'd say either we have a customer linker script for the sample and drop the changes to the generic arm64 linker script or add a Kconfig option that wraps the changes in the linker script.

I dont like the idea of adding something like this and knowing its going to change with @tejlmand changes. Lets either not add it right now or limit it to just the sample at this time.

carlocaione · 2021-07-15T13:03:17Z

So I'd say either we have a customer linker script for the sample and drop the changes to the generic arm64 linker script or add a Kconfig option that wraps the changes in the linker script.

@galak changed to have a modified linker script only for the sample.

Introduce sample application to test reserved-memory helpers. Signed-off-by: Carlo Caione <[email protected]>

carlocaione added the RFC Request For Comments: want input from the community label May 19, 2021

carlocaione requested review from erwango, galak, ioannisg, mbolivar-nordic, nashif, nika-nordic, npitre, pabigot and stephanosio May 19, 2021 14:30

carlocaione requested review from MaureenHelm, andyross, carlescufi and dcpleung as code owners May 19, 2021 14:30

github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree Tooling PR modifies or adds a Device Tree tooling labels May 19, 2021

galak self-assigned this May 19, 2021

ioannisg added this to the v2.7.0 milestone May 19, 2021

mbolivar-nordic reviewed May 19, 2021

View reviewed changes

carlescufi requested review from tejlmand and removed request for pabigot May 20, 2021 11:54

mbolivar-nordic reviewed May 24, 2021

View reviewed changes

include/devicetree.h Outdated Show resolved Hide resolved

include/devicetree.h Outdated Show resolved Hide resolved

dts/bindings/reserved-memory/reserved-memory.yaml Outdated Show resolved Hide resolved

carlocaione requested review from MulinChao, WealianLiao and nvlsianpu as code owners June 23, 2021 06:52

github-actions bot added the area: Tests Issues related to a particular existing or missing test label Jun 23, 2021

mbolivar-nordic approved these changes Jun 29, 2021

View reviewed changes

carlocaione added TSC Topics that need TSC discussion and removed RFC Request For Comments: want input from the community labels Jul 13, 2021

carlocaione changed the title ~~[RFC] Add reserved-memory support~~ Add reserved-memory support Jul 13, 2021

galak reviewed Jul 14, 2021

View reviewed changes

samples/boards/qemu_cortex_a53/reserved_memory/dts/bindings/sample_driver.yaml Outdated Show resolved Hide resolved

galak requested changes Jul 14, 2021

View reviewed changes

carlocaione added 2 commits July 15, 2021 09:15

dts: Introduce DT_STRING_TOKEN and DT_STRING_UPPER_TOKEN

2ed4df3

To be able to get a tokenize DT string without the quotes. Deprecate also DT_ENUM_TOKEN and DT_ENUM_UPPER_TOKEN. Signed-off-by: Carlo Caione <[email protected]>

reserved-memory: Introduce support for reserved-memory node

22f1b07

Introduce a set of header files to be able to define and declare sections and regions in the linker script. Introduce also DT helpers to retrieve data back. Signed-off-by: Carlo Caione <[email protected]>

carlescufi added the dev-review To be discussed in dev-review meeting label Jul 15, 2021

samples: reserved_memory: Introduce sample application

0f9e0e7

Introduce sample application to test reserved-memory helpers. Signed-off-by: Carlo Caione <[email protected]>

carlocaione removed the TSC Topics that need TSC discussion label Jul 15, 2021

galak approved these changes Jul 15, 2021

View reviewed changes

galak removed the dev-review To be discussed in dev-review meeting label Jul 15, 2021

galak merged commit a168454 into zephyrproject-rtos:main Jul 15, 2021

carlocaione deleted the reserved_memory branch July 16, 2021 06:27

carlocaione mentioned this pull request Jul 26, 2021

Add multi-core IPC service #37130

Closed

mbolivar-nordic mentioned this pull request Sep 27, 2021

devicetree/memory.h probably should not exist as-is #38886

Closed

Add reserved-memory support #35460

Add reserved-memory support #35460

Uh oh!

Conversation

carlocaione commented May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbolivar-nordic May 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbolivar-nordic May 19, 2021

Choose a reason for hiding this comment

Uh oh!

carlocaione May 20, 2021

Choose a reason for hiding this comment

Uh oh!

carlescufi May 20, 2021

Choose a reason for hiding this comment

Uh oh!

tejlmand May 27, 2021

Choose a reason for hiding this comment

Uh oh!

galak commented May 20, 2021

Uh oh!

carlocaione commented May 21, 2021

Uh oh!

galak commented May 21, 2021

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlocaione commented May 25, 2021

Uh oh!

JordanYates commented May 25, 2021

Uh oh!

carlocaione commented May 25, 2021

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

carlocaione commented Jul 1, 2021

Uh oh!

carlocaione commented Jul 6, 2021

Uh oh!

carlocaione commented Jul 14, 2021

Uh oh!

Uh oh!

galak left a comment

Choose a reason for hiding this comment

Uh oh!

JordanYates commented Jul 15, 2021

Uh oh!

carlocaione commented Jul 15, 2021

Uh oh!

carlescufi commented Jul 15, 2021

Uh oh!

galak commented Jul 15, 2021

Uh oh!

carlocaione commented Jul 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

carlocaione commented May 19, 2021 •

edited

Loading